fixing the cli option retry_download_count to support simulator by gunesmes · Pull Request #405 · xcpretty/xcode-install

gunesmes · 2020-09-14T15:23:10Z

While I was submitting my PR #404, it was already solved by @freddi-kit with #400. However this PR includes an update for downloading simulators but the corresponding part was not implemented to simulators.rb so this will cause a bug for downloading simulators, as following:

bash-3.2$ xcversion simulators --install='iOS 11.3 Simulator' --retry-download-count=5

simulators --install=iOS 11.3 Simulator --retry-download-count=5
[!] Unknown option: `--retry-download-count=5`
Did you mean: --force?

Usage:

    $ xcversion simulators

      List or install iOS simulators.

Options:

    --install=name   Install simulator beginning with name, e.g. 'iOS 8.4', 'tvOS
                     9.0'.
    --force          Install even if the same version is already installed.
    --no-install     Only download DMG, but do not install it.
    --no-progress    Don’t show download progress.
    --verbose        Show more debugging information
    --no-ansi        Show output without ANSI codes
    --help           Show help banner of specified command
bash-3.2$

or

bash-3.2$ xcversion simulators --install='iOS 11.3 Simulator'

simulators --install=iOS 11.3 Simulator
Installing iOS 11.3 Simulator for Xcode 11.5.0...
Traceback (most recent call last):
	6: from /Users/mesut/.rbenv/versions/2.7.1/bin/xcversion:23:in `<main>'
	5: from /Users/mesut/.rbenv/versions/2.7.1/bin/xcversion:23:in `load'
	4: from /Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/xcode-install-2.6.6/bin/xcversion:12:in `<top (required)>'
	3: from /Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/claide-1.0.3/lib/claide/command.rb:334:in `run'
	2: from /Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/xcode-install-2.6.6/lib/xcode/install/simulators.rb:26:in `run'
	1: from /Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/xcode-install-2.6.6/lib/xcode/install/simulators.rb:45:in `install'
/Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/xcode-install-2.6.6/lib/xcode/install.rb:531:in `install': wrong number of arguments (given 2, expected 3) (ArgumentError)

This PR is fixing this

Simulator

without `--retry_download_count`, default is 3

bash-3.2$ xcversion simulators --install='iOS 11.0 Simulator'

simulators --install=iOS 11.0 Simulator
  9 1989M    9  189M    0     0  1677k      0  0:20:14  0:01:55  0:18:19 1805k%
curl: (18) transfer closed with 1886829804 bytes remaining to read
  5 1799M    5  105M    0     0  1697k      0  0:18:05  0:01:03  0:17:02 1811k%
curl: (18) transfer closed with 1776103408 bytes remaining to read
  9 1693M    9  163M    0     0  1590k      0  0:18:10  0:01:45  0:16:25 1541k%
curl: (18) transfer closed with 1604346613 bytes remaining to read
%[!] Failed to download iOS 11.0 Simulator.

with `--retry-download-count=5` will try to dowload 5 times

bash-3.2$ xcversion simulators --install='iOS 10.3.1 Simulator' --retry=5

simulators --install=iOS 10.3.1 Simulator --retry=5
 13 1198M   13  164M    0     0   717k      0  0:28:30  0:03:54  0:24:36  861k%
curl: (18) transfer closed with 1084795514 bytes remaining to read
 34  865M   34  298M    0     0  1421k      0  0:10:23  0:03:34  0:06:49 1855k%
curl: (18) transfer closed with 595033804 bytes remaining to read
 31  528M   31  163M    0     0  1653k      0  0:05:27  0:01:41  0:03:46 1737k%
curl: (18) transfer closed with 381830355 bytes remaining to read
 45  364M   45  163M    0     0  1194k      0  0:05:12  0:02:20  0:02:52  354k%
curl: (18) transfer closed with 209942487 bytes remaining to read
 53  200M   53  107M    0     0  1510k      0  0:02:15  0:01:12  0:01:03 1760k%
curl: (18) transfer closed with 97659610 bytes remaining to read
%[!] Failed to download iOS 10.3.1 Simulator.
bash-3.2$

gunesmes · 2020-09-14T16:11:19Z

@jpsim @freddi-kit could you please review this?

lib/xcode/install/simulators.rb

lib/xcode/install.rb

freddi-kit · 2020-09-14T17:17:33Z

Thank you for fixing my bug! 👍 I did not notice

lib/xcode/install/simulators.rb

Co-authored-by: freddi(Yuki Aki) <[email protected]>

Co-authored-by: JP Simard <[email protected]>

gunesmes · 2020-09-14T21:02:28Z

@freddi-kit could you please review changes?

freddi-kit · 2020-09-15T00:29:53Z

@gunesmes
Could you help to run bundle exec rubocop -a again? robocop is reporting some code have to be fixed.

lib/xcode/install.rb

gunesmes · 2020-09-15T09:12:46Z

@gunesmes
Could you help to run bundle exec rubocop -a again? robocop is reporting some code have to be fixed.

To solve all offenses, we should have made the number_of_try optional param. I have updated to fix this.
dmg_path = download(progress, nil, number_of_try)

freddi-kit

Great, Thank you!

gunesmes · 2020-09-15T09:43:43Z

@jpsim fixed the issues, could you please accept the pr?

freddi-kit · 2020-09-15T09:59:25Z

@jpsim who can push to gem? it would be helpful the person who have network issue of curl.

traviswimer · 2020-10-04T15:22:41Z

Is there something holding up this pull-request? It sounds like the work has been completed.

I tried just cloning the repo and running from this branch, but I can't seem to get it to work. I think it's likely my lack of Ruby knowledge is the problem though. I get an error running bundle install that it needs an older version of bundler, but using the older version tells me it needs the newer version.

If this isn't ready to be merged, could someone offer some guidance on how I can get this branch working as a temporary fix?

gunesmes · 2020-10-05T17:39:36Z

@traviswimer someone who have write access should approve this PR and then push the gem for the new release. @joshdholtz @KrauseFx @orta or @steipete

joshdholtz · 2020-10-05T17:43:08Z

@gunesmes On it! And just for record... I’m the only one out of those 4 mentioned that are associated with this project ATM so no need to mention them in the future 😉 Just trying to save everyone from some extra GitHub notifications 🙃

joshdholtz

@gunesmes One small question before approving/merging! And thanks for thIs PR and the ping ❤️

joshdholtz · 2020-10-05T17:56:11Z

lib/xcode/install/install.rb

         ['--no-clean', 'Don’t delete DMG after installation.'],
         ['--no-show-release-notes', 'Don’t open release notes in browser after installation.'],
-         ['--retry-download-count', 'Count of retrying download when curl is failed.']].concat(super)
+         ['--number-of-try', 'How many times try to download DMG file if downloading fails. Default is 3.']].concat(super)


I think I’m confused on what the reasoning for this rename is. This is technically a breaking change, isn’t it? Was there some issue with retry-download-count?

If anything, I would maybe recommend this to be named retry-count. Thoughts?

I thought that this would be better for grammar.

I changed the parameter to retry-count

Thank you! I will pull down and play with one more time after I’m done taking the doggo for a walk 😊

Thanks again for this PR! I’ll get it release in an hour or so if all is 💯 (which is looks like it should be)

joshdholtz

I think the fetching of options need to be renamed too? 🤔 After this we should be good!

lib/xcode/install/simulators.rb

lib/xcode/install/install.rb

Co-authored-by: Josh Holtz <[email protected]>

joshdholtz

One more thing! I think we want to keep the --retry that was getting passed in to curl

The return around curl is a redundancy around the curl retry I believe 😇

lib/xcode/install.rb

…//github.com/gunesmes/xcode-install into fix/extending-cli-option-retry-for-simulator

gunesmes · 2020-10-06T15:04:48Z

@joshdholtz to fix the Assignment Branch Condition, separated the curl part of the function to new function as curl_file.

joshdholtz · 2020-10-06T17:35:38Z

@gunesmes Thank you! Testing this out one more time by downloading some Xcode but code is looking 💯

joshdholtz

Downloading seems good! But had an error installing 🤔

lib/xcode/install.rb

joshdholtz · 2020-10-06T21:41:29Z

@gunesmes What is this PR originally just for? Was this just for really adding the --retry-count to the simulators command? 😇

gunesmes · 2020-10-06T21:45:23Z

Yes this is fixing the simulator part for adding retry-count.

joshdholtz · 2020-10-06T21:56:24Z

@gunesmes Okay, thank you! I’ll dig in to see what’s going on. I might end up redoing some of the argument name changes to make it backwards compatible with the previous version but I will make sure that parameter works 😇 Hopefully we can get this merged and released for you tomorrow!

freddi-kit · 2020-12-03T02:15:08Z

How is this progressed?

gunesmes · 2020-12-06T15:48:13Z

@joshdholtz do you have any update for the your testing?

pronebird · 2021-05-22T10:26:13Z

Can we please bump the default retry count to 10 because there is no way this succeeds in three attempts.

But on the other side, it looks like xcversion does not really understand what's happening in curl. I still see this as a bug, because it's clear that curl knows that there is more data left to download, yet it stops for no apparent reason.

I think the code handling the retry count should be fixed, i.e:

let CURLE_PARTIAL_FILE = 18
if curl.exitCode == CURLE_PARTIAL_FILE, downloadFileSize < expectedFileSize {
  continueDownload()
} else {
  retryCount += 1
  if retryCount > maxRetryCount {
    reportFailure()
  } else {
    continueDownload()
  } 
}

That's the thing I continue witnessing for years now:

xcversion simulators --install="iOS 10.3.1"          

Installing iOS 10.3.1 Simulator for Xcode 12.4.0...
 13 1883M   13  249M    0     0  2882k      0  0:11:09  0:01:28  0:09:41 2562k%
curl: (18) transfer closed with 1713831788 bytes remaining to read
 22 1634M   22  361M    0     0  2475k      0  0:11:16  0:02:29  0:08:47 2399k%
curl: (18) transfer closed with 1334473327 bytes remaining to read
 12 1272M   12  165M    0     0  3026k      0  0:07:10  0:00:55  0:06:15 3755k%^R
 24 1272M   24  305M    0     0  2764k      0  0:07:51  0:01:53  0:05:58 2404k%
curl: (18) transfer closed with 1013687667 bytes remaining to read
%[!] Failed to download iOS 10.3.1 Simulator.

gunesmes · 2021-05-22T16:40:28Z

Update retrial count to 10

Onno113 · 2021-08-10T15:02:39Z

I am not sure what is missing to put the changes here to a release. It looks like it could be missing some kind of approval from @joshdholtz

I am trying to install simulators with xcversion, but it fails every time. I tried already around 10 times. I would like this to be more reliable, because we want to use this for the setup of our CI machines.

xcversion is an amazing tool and it helps us already a lot with installing different kind of Xcode versions, thanks a lot!
:-)

gunesmes · 2022-10-11T13:32:55Z

@joshdholtz this is still waiting for your approval, any chance you can review it?

fixing the cli option retry_download_count to support simulator

6c56bc6

jpsim approved these changes Sep 14, 2020

View reviewed changes

lib/xcode/install/simulators.rb Outdated Show resolved Hide resolved

freddi-kit reviewed Sep 14, 2020

View reviewed changes

lib/xcode/install.rb Show resolved Hide resolved

freddi-kit reviewed Sep 14, 2020

View reviewed changes

lib/xcode/install/simulators.rb Outdated Show resolved Hide resolved

gunesmes and others added 5 commits September 14, 2020 23:02

Update lib/xcode/install/simulators.rb

3043349

Co-authored-by: freddi(Yuki Aki) <[email protected]>

Update lib/xcode/install/simulators.rb

0f96458

Co-authored-by: JP Simard <[email protected]>

change option name to number-of-try

4346244

change option name to number-of-try

a44cd49

change option name to number-of-try

0416bc1

freddi-kit reviewed Sep 15, 2020

View reviewed changes

lib/xcode/install.rb Outdated Show resolved Hide resolved

gunesmes added 2 commits September 15, 2020 09:29

change initial value of number_of_try

094549d

rubocop auto-fixes

1e19ac4

freddi-kit reviewed Sep 15, 2020

View reviewed changes

lib/xcode/install.rb Outdated Show resolved Hide resolved

freddi-kit reviewed Sep 15, 2020

View reviewed changes

lib/xcode/install.rb Outdated Show resolved Hide resolved

fixed rubocop offenses

323dff9

freddi-kit approved these changes Sep 15, 2020

View reviewed changes

gunesmes mentioned this pull request Sep 16, 2020

Xcode download fails with "curl: (18)" #390

Open

joshdholtz requested changes Oct 5, 2020

View reviewed changes

gunesmes added 2 commits October 5, 2020 21:12

rename parameter

fdc2749

rename parameter

c14e5d2

joshdholtz requested changes Oct 5, 2020

View reviewed changes

lib/xcode/install/simulators.rb Outdated Show resolved Hide resolved

lib/xcode/install/install.rb Outdated Show resolved Hide resolved

gunesmes and others added 2 commits October 5, 2020 23:23

Update lib/xcode/install/simulators.rb

6390847

Co-authored-by: Josh Holtz <[email protected]>

Update lib/xcode/install/install.rb

d77e6b8

Co-authored-by: Josh Holtz <[email protected]>

joshdholtz requested changes Oct 5, 2020

View reviewed changes

lib/xcode/install.rb Show resolved Hide resolved

gunesmes added 2 commits October 6, 2020 15:06

readding the retry_options

65d6900

Merge branch 'fix/extending-cli-option-retry-for-simulator' of https:…

5c39dc5

…//github.com/gunesmes/xcode-install into fix/extending-cli-option-retry-for-simulator

gunesmes requested a review from joshdholtz October 6, 2020 12:14

gunesmes added 2 commits October 6, 2020 16:02

rubocup fix

604d308

rubocup fix Assignment Branch Condition

0909d80

joshdholtz requested changes Oct 6, 2020

View reviewed changes

lib/xcode/install.rb Show resolved Hide resolved

freddi-kit mentioned this pull request Oct 16, 2020

Add --retry-download-count for retrying installing Xcode xip/dmg #400

Merged

Merge branch 'master' into fix/extending-cli-option-retry-for-simulator

dbacf6e

gunesmes added 3 commits May 22, 2021 19:31

update default number of trial to 10

3c92030

update default retrial count to 10

e27710a

update retrial count to 10

3062bd2

gunesmes requested a review from joshdholtz May 27, 2021 12:52

Conversation

gunesmes commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Simulator

without --retry_download_count, default is 3

with --retry-download-count=5 will try to dowload 5 times

Uh oh!

gunesmes commented Sep 14, 2020

Uh oh!

Uh oh!

Uh oh!

freddi-kit commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

gunesmes commented Sep 14, 2020

Uh oh!

freddi-kit commented Sep 15, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gunesmes commented Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

freddi-kit left a comment

Choose a reason for hiding this comment

Uh oh!

gunesmes commented Sep 15, 2020

Uh oh!

freddi-kit commented Sep 15, 2020

Uh oh!

traviswimer commented Oct 4, 2020

Uh oh!

gunesmes commented Oct 5, 2020

Uh oh!

joshdholtz commented Oct 5, 2020

Uh oh!

joshdholtz left a comment

Choose a reason for hiding this comment

Uh oh!

joshdholtz Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

gunesmes Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

gunesmes Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

joshdholtz Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

joshdholtz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joshdholtz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gunesmes commented Oct 6, 2020

Uh oh!

joshdholtz commented Oct 6, 2020

Uh oh!

joshdholtz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joshdholtz commented Oct 6, 2020

Uh oh!

gunesmes commented Oct 6, 2020

Uh oh!

joshdholtz commented Oct 6, 2020

Uh oh!

freddi-kit commented Dec 3, 2020

Uh oh!

gunesmes commented Dec 6, 2020

Uh oh!

gunesmes commented Sep 14, 2020 •

edited

Loading

without `--retry_download_count`, default is 3

with `--retry-download-count=5` will try to dowload 5 times

freddi-kit commented Sep 14, 2020 •

edited

Loading

gunesmes commented Sep 15, 2020 •

edited

Loading

pronebird commented May 22, 2021 •

edited

Loading